Skip to content

Feat: EmitterMeshFaceShape - enhance particle emission uniformity #2503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

capdevon
Copy link
Contributor

@capdevon capdevon commented Jun 18, 2025

This PR introduces significant improvements to the EmitterMeshFaceShape class. The core change involves updating the triangle point generation algorithm to use barycentric coordinates, ensuring a more uniform and accurate distribution of particles across mesh faces. The barycentric coordinate method is more standard and often easier to reason about for uniform distribution within a triangle.

Additionally, this PR includes:

  • Improved Documentation: Detailed Javadoc comments have been added throughout the class, providing clearer explanations of its functionality, constructors, and methods for better code readability and maintainability.

Red Arrow: old algorithm (linear interpolation)
Green Arrow: barycentric coordinates

image

@yaRnMcDonuts yaRnMcDonuts added this to the v3.9.0 milestone Jun 19, 2025
@yaRnMcDonuts
Copy link
Member

yaRnMcDonuts commented Jul 4, 2025

Red Arrow: old algorithm (linear interpolation)
Green Arrow: barycentric coordinates

I wonder if it may be best to make an option to let the developer choose between the old and new algorithm, that way existing particle effects won't be changed.

@capdevon
Copy link
Contributor Author

capdevon commented Jul 7, 2025

I believe it's unnecessary to expend additional time and maintenance on an outdated, obsolete algorithm. The new algorithm is demonstrably clearer and more efficient, offering significant improvements that will ultimately benefit all developers.

My focus is on providing the best possible tools, and maintaining two separate algorithms would divert resources that could be better spent on further enhancing the new solution. I'm confident that transitioning to the new algorithm will lead to a more streamlined and performant experience for everyone.

If you disagree with my implementation, I can close this PR and create my own EmitterShape class, since the JME architecture allows it. However, this would result in the loss of this functionality for the JME community.

@yaRnMcDonuts
Copy link
Member

yaRnMcDonuts commented Jul 7, 2025

My worry is that the differing behavior may result in existing particle effects behaving different visually after this change.

This may cause an app with a well-tweaked particle emitter to now have that particle emitter behaving differently. Whether that current algorithm is right/wrong or better/worse is not as important as making sure that existing particle emitters still behave the way the developer expects.

I'd have to see a side-by-side comparison of a few particle emitters with the new compared to old algorithm to see if the visual difference is noticeable enough that it may break existing particle effects.

MY concerns may be overly cautious. But maybe @riccardobl has some insight to offer since he has more experience than I do working with the backend of particle emitters.

@riccardobl
Copy link
Member

riccardobl commented Jul 12, 2025

If the emission was deterministic i would have suggested to deprecate the old class and to implement a new shape, but it is not, so imo in this case we can merge the patch as it is.

I see this more as a bug fix, because someone using this emission shape is already expecting a random uniform emission anyway, the only difference is that the previous one was not so uniform.

@yaRnMcDonuts yaRnMcDonuts merged commit 4db274c into jMonkeyEngine:master Jul 13, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants